-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tooling for Mac Distribution #324
base: main
Are you sure you want to change the base?
Conversation
|
||
By default, notarization is disabled and will output dryrun logs. To enable it you must either set the following options: | ||
```shell | ||
mac-distribution --apple-username="" --apple-password="" --signing-identity="" --bundle-id="" ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we support flags at all? Doesn't this just make it more likely that secrets get recorded in shell history files?
However this isn't desirable in CI environments where notarization must happen. Enabling dryrun will "silently" cause a failure. | ||
For convenience the `--force-notarization` flag is provided to fail in the scenario where creds are missing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most CI environments set a CI
environment variable - what do you think about automatically forcing notarization when $CI
is set?
|
||
r, err := os.OpenFile(src, os.O_RDONLY, 0) | ||
if err != nil { | ||
return trace.Wrap(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On error should we be deleting the dst file or leaving it behind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to attempt deletion on encountering an error.
|
||
App Bundle (.app) | ||
```shell | ||
mac-distribution package-app tsh tsh.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having a separate tool for every artifact, what do you think about having a single build tool with subcommands that we can extend as we convert more stuff? E.g. something like:
$ tbuild build-mac ... --notarize
|
||
// The Apple Notary Service requires a zip file for notarization. | ||
// The files will be staged in a temporary directory and zipped for submission. | ||
notaryDir, err := os.MkdirTemp("", "notarize-binaries-*") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we include something in the dir name to differentiate between different builds? Like a version number? Just for potential debugging/troubleshooting.
// The Apple Notary Service requires a zip file for notarization. | ||
// The files will be staged in a temporary directory and zipped for submission. | ||
notaryDir, err := os.MkdirTemp("", "notarize-binaries-*") | ||
if err != nil { | ||
return trace.Wrap(err) | ||
} | ||
defer os.RemoveAll(notaryDir) | ||
|
||
// Stage files for notarization | ||
for _, file := range files { | ||
dest := filepath.Join(notaryDir, filepath.Base(file)) | ||
if err := fileutil.CopyFile(file, dest); err != nil { | ||
return trace.Wrap(err) | ||
} | ||
} | ||
|
||
// Zip the staged directory where the binaries are located | ||
notaryfile, err := os.CreateTemp("", "notarize-binaries-*.zip") | ||
if err != nil { | ||
return trace.Wrap(err) | ||
} | ||
defer notaryfile.Close() | ||
defer os.Remove(notaryfile.Name()) | ||
|
||
if err := t.zip.ZipDir(notaryDir, notaryfile, zipper.DirZipperOpts{}); err != nil { | ||
return trace.Wrap(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're doing all this with Go code now, I feel it is unnecessary to create a temp dir and copy all the files into there to zip them. We can create zip files using archive/zip.Writer.CreateHeader
to specify the filename inside the zip file without needing the files to be in a similar place. Perhaps this is a future optimization though - I'll leave that up to you, but I think we should do this as it is a lot less copying of bytes, and less clean-up that may go wrong leaving garbage.
* Copying files preserves perms by default and removes dest on errors * Appbundle sets perms for binary correctly * Copy should properly propagate Close errors * Notarize binaries no longer creates a staging dir for zipping * Moving around packaging files * Unexporting submissionResponseData * Using byte slice for stdout * Fixing zipper archiveName * Cleaning up notarytool * Cleaning up some packaging names * Cleaning up main
No description provided.